-
Notifications
You must be signed in to change notification settings - Fork 31
Remove $facet in top level group stages #449
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Remove $facet in top level group stages #449
Conversation
|
Awesome, thank you and thanks @asya999 |
aclark4life
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a test, particular one that would test something that breaks in the Django admin with QE (even though QE is not merged yet)?
django_mongodb_backend/compiler.py
Outdated
| } | ||
| ) | ||
| pipeline.append({"$group": group}) | ||
| # It may be a bug, $$NOW has to be called to be reachable in the rest of the pipeline. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add link to the SERVER ticket and add an explanatory comment about the purpose of the $unionWith clause.
I think we can also mention it in the release notes as a performance improvement since the team said $facet prevents the use of optimizations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sadly, this cannot be implemented as it is.
To handle the global aggregation and the default values (for example:
SELECT coalesce(avg(age), 35) FROM people WHERE age > 100;we need to wrap the aggregation so MongoDB always returns a single document.
This is done by wrapping the pipeline inside a lookup. Example:
db.z.aggregate([
{ $collStats: {} },
{ $lookup: { from: "z", as: "wrap", pipeline: [...pipeline stages] } },
{ $replaceWith: {"$cond": [{"$eq": ["$wrap", []]}, {}, {$first: "$wrap"}]}}
])
It’s not very intuitive use of $collStats, but it works 😄. It replaces the need for $unionWith.
$collStats is used as a pivot so we can apply the required operator after the group result.
|
Can you provide, in this PR, an example of what the removal of $facet makes the new query signatures look like? |
3e2c32d to
273957f
Compare
Generated query:
[{'$collStats': {}}, {'$lookup': {'as': 'wrapped', 'from': 'aggregation_author', 'pipeline': [{'$group': {'_id': None, 'count': {'$sum': {'$cond': {'else': 1, 'if': {'$in': [{'$type': '$_id'}, ['missing', 'null']]}, 'then': None}}}}}]}}, {'$replaceWith': {'$cond': [{'$eq': ['$wrapped', []]}, {}, {'$first': '$wrapped'}]}}, {'$project': {'count': {'$ifNull': ['$count', {'$literal': 0}]}}}]